-
-
Notifications
You must be signed in to change notification settings - Fork 1k
Add react-native-svg interface
#3242
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
react-native-svg for improved hit detection of svg elementsreact-native-svg to improve hit-boxes of svg elements
react-native-svg to improve hit-boxes of svg elementsreact-native-svg interface
|
Integration works as of bd51ac7, but I have to test it a bit more before it's ready. |
…interface (#2555) <!-- Thanks for submitting a pull request! We appreciate you spending the time to work on these changes. Please follow the template so that the reviewers can easily understand what the code changes affect --> # Summary Adding an `RNGH <-> RNSVG` interface requires usage of `RenderableView.hitTest` to work. ([link](software-mansion/react-native-gesture-handler#3242)) Currently, `RenderableView.hitTest` is `package-private`, meaning it cannot be accessed by other packages. This change does not change any functionality of the library, it only exposes existing functions to other libraries. I only made public those `hitTest` implementation which are strictly necessary for this interface, this is why multiple, if not most `hitTest` implementations remain `package-private` despite the changes made in the PR. ## Test Plan - open the example app, see how the app builds successfully ### What's required for testing (prerequisites)? - `RNSVG`'s `paper-example` app ### What are the steps to reproduce (after prerequisites)? ## Compatibility | OS | Implemented | | ------- | :---------: | | iOS | ❌ | | MacOS | ❌ | | Android | ✅ | | Web | ❌ | ## Checklist <!-- Check completed item, when applicable, via: [X] --> - [X] I have tested this on a device and a simulator - [ ] I added documentation in `README.md` - [ ] ~~I updated the typed files (typescript)~~ - [ ] I added a test for the API in the `__tests__` folder
…tware-mansion/react-native-gesture-handler into @latekvo/add-svg-integration
android/svg/src/main/java/com/swmansion/gesturehandler/RNSVGHitTester.kt
Show resolved
Hide resolved
android/nosvg/src/main/java/com/swmansion/gesturehandler/RNSVGHitTester.kt
Outdated
Show resolved
Hide resolved
<!-- Thanks for submitting a pull request! We appreciate you spending the time to work on these changes. Please follow the template so that the reviewers can easily understand what the code changes affect --> # Summary This PR reverts #2555, as `RenderableView.hitTest()` can be replaced by `SvgView.reactTagForTouch()`, which is already `public`. This PR also changes the `package-private` `getSvgView()` method of `VirtualView` to `public`. This change has been made to handle hit detection of transformed `VirtualView`s, as `RenderableView`'s `hitTest()` doesn't take transformations into account, while `SvgView`'s `reactTagForTouch()` does. Making `getSvgView()` public is necessary for integrating RNSVG support into RNGH. More details [here](software-mansion/react-native-gesture-handler#3242). ## Test Plan Run the example from the RNGH - RNSVG [integration PR](software-mansion/react-native-gesture-handler#3242). ## Compatibility | OS | Implemented | | ------- | :---------: | | Android | ✅ |
…tware-mansion/react-native-gesture-handler into @latekvo/add-svg-integration
| "[react-native-gesture-handler] Unable to resolve react-native location in " + | ||
| "node_modules. You should add project extension property (in app/build.gradle) " + | ||
| "`REACT_NATIVE_NODE_MODULES_DIR` with path to react-native." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this done by a formatter?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the formatter.
I debated myself about removing these changes, but figured there's no benefit for deferring them, or opening a separate PR for them.
| return (Integer.parseInt(major) == 15 && Integer.parseInt(minor) == 11 && Integer.parseInt(patch) >= 2) || | ||
| (Integer.parseInt(major) == 15 && Integer.parseInt(minor) > 11) || | ||
| Integer.parseInt(major) > 15 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment above states that it is available from 15.11.0, but here we check for 15.11.2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
android/build.gradle
Outdated
| exclude group: 'com.facebook.fbjni' | ||
| // resolves "Duplicate class com.facebook.jni.CppException" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is also done by formatter, could you please move this comment line above?
Also, what formatter do you use? I do not recall that our formatting script changes gradle files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right that :spotlessApply has no effect on gradle files. I ran the built-in android studio Groovy formatter, to format the version-checking part of shouldUseCommonInterfaceFromRNSVG.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moved comment in 423ac1c
Description
This PR adds integration with the
react-native-svg(RNSVG) to improve hitbox detection of the SVG elements they provide.blockedby software-mansion/react-native-svg#2583blockedbynested SvgViews with viewBox prop have invalid hit detection- No issue opened yetTest plan